refactor(abstract-eth): move txBuilder to abstract-eth #4030

gianchandania · 2023-10-31T13:06:45Z

The main purpose of this PR is to move TransactionBuilder and other related classes from sdk-coin-eth to abstract-eth module. This is being done as part of refactoring effort to make it easy to add new eth-like chains.

This PR has 8 commits

This commit moves the transaction builder from sdk-coin-eth to abstract-eth module. As part of this commit we are avoiding to make any imports from sdk-coin-eth in abstract-eth module and instead moving all the required classes and methods in abstract-eth module. This commit also adds a new class AbstractEthLikeNewCoins. This class will be inherited by all the eth-like coins like polygon, arbitrum, optimism, bsc
This commit makes changes in sdk-coin-polygon related to txBuilder refactor.
This commit
makes changes in sdk-coin-arbeth related to txBuilder refactor. It adds getContractData method for arbeth. It also adds bytecode and abi for the ArbethWalletSimple contract
This commit
makes changes in sdk-coin-opeth related to txBuilder refactor. It adds getContractData method for arbeth. It also adds bytecode and abi for the OpethWalletSimple contract
This commit makes changes in sdk-coin-ethw related to txBuilder refactor.
This commit fixes the issue related to bignumber package versions
This commit deletes the MPC related classes as we will use AbstractEthLikeNewCoins class for multisig and MPC related methods for all the upcoming EthLike coins
This commit exports Ethereum Transactionbuilder and Transfer builder as this was the behavior before this change

WIN-1021
TICKET: WIN-1021

socket-security · 2023-10-31T13:52:54Z

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎

gianchandania · 2023-10-31T19:31:08Z

modules/abstract-eth/src/ethLikeToken.ts

-  protected getTransactionBuilder(): EthTransactionBuilder {
-    return new EthTransactionBuilder(coins.get(this.getBaseChain()));
-  }
-


Modified this method because TransactionBuilder in abstract-eth is an abstract class and we can't instantiate the abstract class

rushilbg

The commits linked in the description are invalid.

gianchandania · 2023-11-01T07:12:59Z

The commits linked in the description are invalid.

sorry for that, have updated the links in the description

sachushaji

Nice Job!!! ❤️ ❤️ , This will speed up EVM based onboarding onto SDK for future coins!! , just have some small comments

sachushaji · 2023-11-01T12:25:15Z

modules/sdk-coin-eth/test/unit/getBuilder.ts

@@ -1,4 +1,4 @@
-import { TransactionBuilder } from '../../src';
+import { TransactionBuilder } from '../../src/lib/transactionBuilder';


Think we need to keep the import as is?

This was by mistake, ideally the previous import is working fine, will address this in a follow up

sachushaji · 2023-11-01T14:32:48Z

modules/sdk-coin-opeth/src/lib/transactionBuilder.ts

+    const resultEncodedParameters = EthereumAbi.rawEncode(walletSimpleConstructor, params)
+      .toString('hex')
+      .replace('0x', '');
+    return walletSimpleByteCode + resultEncodedParameters;


May be we can add it to a property / method in the class to get this value which can help us abstract it further and avoid the need of redefining it each time

I will address this in a follow up PR as part of this ticket WIN-1012

sachushaji · 2023-11-01T14:34:06Z

modules/sdk-coin-opeth/test/unit/opeth.ts

@@ -23,7 +23,6 @@ describe('Optimism', function () {
      opeth.getFamily().should.equal('opeth');
      opeth.getFullName().should.equal('Optimism Ethereum');
      opeth.getBaseFactor().should.equal(1e18);
-      opeth.supportsTss().should.equal(true);


zahin-mohammad

Not sure why I am needed as a review for this, but we are likely missing some code owner rules.

WIN-1021 BREAKING CHANGE: getTransactionBuilder method is removed from EthLikeToken class in abstract-eth module because TransactionBuilder in the abstract-eth module is abstract class and hence cannot be instantiated. Hence the implementation of TransactionBuilder can be added to the class that will inherit EthLikeToken class TransactionPrebuild from new class AbstractEthLikeNewCoins is being exported now instead of TransactionPrebuild from AbstractEthLikeCoin class as the TransactionPrebuild from AbstractEthLikeNewCoins also has support for hop transactions, batch transactions, etc TICKET: WIN-1021

TICKET: WIN-1021

WIN-1021 BREAKING CHANGE: AbstractEthLikeMPCCoin and EthLikeMPCToken classes are removed as we have instead added a new class AbstractEthLikeNewCoins which will be having both multisig and MPC related methods TICKET: WIN-1021

WIN-1021 TICKET: WIN-1021

gianchandania force-pushed the WIN-1021-move-txbuilder-abstracteth branch 7 times, most recently from 34d1c76 to baec169 Compare October 31, 2023 19:28

gianchandania commented Oct 31, 2023

View reviewed changes

gianchandania force-pushed the WIN-1021-move-txbuilder-abstracteth branch 4 times, most recently from c74f685 to 8548afb Compare November 1, 2023 06:51

gianchandania marked this pull request as ready for review November 1, 2023 06:53

gianchandania requested review from a team as code owners November 1, 2023 06:53

rushilbg reviewed Nov 1, 2023

View reviewed changes

gianchandania requested a review from rushilbg November 1, 2023 07:13

sachushaji reviewed Nov 1, 2023

View reviewed changes

zahin-mohammad approved these changes Nov 1, 2023

View reviewed changes

gianchandania added 6 commits November 2, 2023 09:11

refactor(sdk-coin-polygon): make changes related to txbuilder refactor

8b09148

TICKET: WIN-1021

refactor(sdk-coin-arbeth): make changes related to txbuilder refactor

0460371

TICKET: WIN-1021

refactor(sdk-coin-opeth): make changes related to txbuilder refactor

1288d10

TICKET: WIN-1021

refactor(sdk-coin-ethw): make changes related to txbuilder refactor

0933e3e

TICKET: WIN-1021

fix(sdk-core): fix issue related to bignumber version

519fe47

TICKET: WIN-1021

gianchandania added 2 commits November 2, 2023 09:11

refactor(abstract-eth): delete mpc related classes

52396ed

WIN-1021 BREAKING CHANGE: AbstractEthLikeMPCCoin and EthLikeMPCToken classes are removed as we have instead added a new class AbstractEthLikeNewCoins which will be having both multisig and MPC related methods TICKET: WIN-1021

fix(sdk-coin-eth): fix issue related to ethtxbuilder

286ccfd

WIN-1021 TICKET: WIN-1021

gianchandania force-pushed the WIN-1021-move-txbuilder-abstracteth branch from 8548afb to 286ccfd Compare November 2, 2023 03:41

sachushaji approved these changes Nov 2, 2023

View reviewed changes

gianchandania merged commit 20e4cf4 into master Nov 2, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(abstract-eth): move txBuilder to abstract-eth #4030

refactor(abstract-eth): move txBuilder to abstract-eth #4030

gianchandania commented Oct 31, 2023 •

edited

Loading

socket-security bot commented Oct 31, 2023 •

edited

Loading

gianchandania Oct 31, 2023

rushilbg left a comment

gianchandania commented Nov 1, 2023

sachushaji left a comment •

edited

Loading

sachushaji Nov 1, 2023

gianchandania Nov 2, 2023 •

edited

Loading

sachushaji Nov 1, 2023

gianchandania Nov 2, 2023

sachushaji Nov 1, 2023

zahin-mohammad left a comment

		@@ -1,4 +1,4 @@
		import { TransactionBuilder } from '../../src';
		import { TransactionBuilder } from '../../src/lib/transactionBuilder';

refactor(abstract-eth): move txBuilder to abstract-eth #4030

refactor(abstract-eth): move txBuilder to abstract-eth #4030

Conversation

gianchandania commented Oct 31, 2023 • edited Loading

socket-security bot commented Oct 31, 2023 • edited Loading

gianchandania Oct 31, 2023

Choose a reason for hiding this comment

rushilbg left a comment

Choose a reason for hiding this comment

gianchandania commented Nov 1, 2023

sachushaji left a comment • edited Loading

Choose a reason for hiding this comment

sachushaji Nov 1, 2023

Choose a reason for hiding this comment

gianchandania Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

sachushaji Nov 1, 2023

Choose a reason for hiding this comment

gianchandania Nov 2, 2023

Choose a reason for hiding this comment

sachushaji Nov 1, 2023

Choose a reason for hiding this comment

zahin-mohammad left a comment

Choose a reason for hiding this comment

gianchandania commented Oct 31, 2023 •

edited

Loading

socket-security bot commented Oct 31, 2023 •

edited

Loading

sachushaji left a comment •

edited

Loading

gianchandania Nov 2, 2023 •

edited

Loading